Add Data - CSV Upload UI #6845

Bargs · 2016-04-09T23:13:55Z

Fixes #6541
Depends on #6844 and #6842

This PR implements the CSV Upload Add Data Wizard. Some of the highlights of the changes:

A new upload-wizard directive which acts as the container for the CSV wizard. This intentionally duplicates some code and structure from the Tail a File wizard to avoid prematurely DRYing things up before we really know exactly what a wizard API needs to look like.
A new parse-csv-step directive which allows a user to pick and preview their CSV
A new upload-data-step directive for uploading the CSV to the Kibana _data API and displaying the results to the user
A new uploadCSV method on the ingest service, for sending File objects to the Kibana _data API.

The CSV wizard does not currently include a pipeline creation step. It will be re-added once we have editable pipelines implemented.

rashidkpc · 2016-04-14T20:36:00Z

jenkins, test it

rashidkpc · 2016-04-14T22:42:59Z

@tsullivan can you look at the test failure here? It seems to be related to the uuid code

tsullivan · 2016-04-14T23:50:07Z

Looking into it. I get the failure locally too

tsullivan · 2016-04-15T01:29:48Z

@rashidkpc looks like all the tests that use this mechanism cause a failure:

    kbnServer = kbnTestServer.createServer({
      plugins: {
        scanDirs: [
          fromRoot('src/plugins')
        ]
      }
    });

The server test suite only passes if I skip all these:

src/plugins/elasticsearch/lib/__tests__/manage_uuid.js
src/plugins/elasticsearch/lib/__tests__/routes.js
src/server/http/__tests__/index.js

My theory is that the scanDirs/fromRoot setup is calls some constructor function without the new keyword, which causes context to be bound to the global context. But I'm still not really sure. Still looking.

tsullivan · 2016-04-15T04:47:37Z

@Bargs looks like the highland module is purposefully creating a global variable called nil and that's why mocha is failing with Error: global leak detected: nil

Source: https://github.com/caolan/highland/blob/master/dist/highland.js#L177

Looks like there is a way to register globals via config with grunt-simple-mocha: https://github.com/yaymukund/grunt-simple-mocha

tsullivan · 2016-04-21T22:01:14Z

Has conflicts that need to be resolved

Bargs · 2016-04-27T23:05:47Z

Thanks @tsullivan, adding nil to the list of globals fixed the issue.

Bargs · 2016-05-12T21:01:02Z

jenkins, test it

BigFunger · 2016-05-25T22:08:25Z

.../kibana/public/settings/sections/indices/add_data_steps/upload_data_step/upload_data_step.js

@@ -28,7 +28,7 @@ modules.get('apps/settings')
          _.forEach(res.data, (response) => {
            this.created += response.created;
            this.formattedErrors = this.formattedErrors.concat(_.map(_.get(response, 'errors.index'), (doc) => {
-              return `${doc._id.split('-', 1)[0].replace('L', 'Line ').trim()}: ${doc.error.type} - ${doc.error.reason}`;
+              return `Line ${doc._id.substr(doc._id.lastIndexOf(':') + 1)}: ${doc.error.type} - ${doc.error.reason}`;


Is it worth pulling this logic into a function? Especially since the format of the data has changed now during the development of this feature?

BigFunger · 2016-05-25T22:09:08Z

One more small suggested change, not a deal breaker. Once tests pass, LGTM.

Bargs · 2016-05-25T22:11:28Z

jenkins, test it

… public contract

Bargs · 2016-05-25T22:38:18Z

@BigFunger

Extracted error formatting into its own function
Added an API test for ID format

BigFunger · 2016-05-26T14:55:28Z

LGTM

BigFunger · 2016-05-26T19:56:51Z

Still LGTM. Thanks for addressing the duplicate column issue.

w33ble · 2016-05-27T21:07:41Z

I have a CSV file with 2,600+ columns and 68,000+ rows. Importing that seems to blow up the parser or something.

Also, apparently Elasticsearch doesn't allow that many fields on a document:

Pretty edge-case, I know, just wanted to share.

w33ble · 2016-05-27T22:03:04Z

@Bargs since you seemed to think that the large file importing was caused by misuse of papaparse, I'm sending back to you to fix that.

OTT, LGTM!

Previously I was using PapaParse's preview option, but it turns out that does not prevent the library from loading the entire file if you don't use one of the streaming callbacks. So now we have to do some extra gymnastics to gather the data per row in the step callback, manually abort parsing if we haven't hit the end of the file after a given number of sample lines and then digest the data in the complete callback. I also added an extra condition to a watcher that caused parsing to happen twice.

Bargs · 2016-06-01T00:05:09Z

@w33ble just pushed some performance improvements. Really glad you caught this issue. After profiling the code I realized there were a combination of things dragging performance down. Give it another whirl and tell me what you think. I was able to add your 700MB CSV to the first wizard step in about 8 seconds without the browser choking. As far as I can tell, the bottleneck is Angular now. With all those columns, it's having to create a lot of DOM elements. If you have any suggestions on optimizing that, I'd be all ears!

…ance degrades if the user has hundreds or thousands

w33ble · 2016-06-01T21:49:13Z

Bargs assigned rashidkpc Apr 9, 2016

Bargs added review Feature:Add Data Add Data and sample data feature on Home labels Apr 9, 2016

This was referenced Apr 9, 2016

[FileUpload] Enhance file-upload directive #6842

Merged

[API] Add CSV bulk indexing support to Kibana API #6844

Merged

Bargs force-pushed the ingest/uploadUI branch from 951264a to 2f8aa4b Compare April 11, 2016 23:00

rashidkpc assigned tsullivan and unassigned rashidkpc Apr 14, 2016

tsullivan assigned Bargs and unassigned tsullivan Apr 25, 2016

Bargs force-pushed the ingest/uploadUI branch 2 times, most recently from 3588918 to cad25fc Compare April 29, 2016 16:14

Bargs removed the review label Apr 29, 2016

Bargs force-pushed the ingest/uploadUI branch from cad25fc to 868bee4 Compare April 29, 2016 22:07

Bargs force-pushed the ingest/uploadUI branch 4 times, most recently from 5643e53 to e9a5ebe Compare May 12, 2016 16:23

Bargs force-pushed the ingest/uploadUI branch from e9a5ebe to bed8725 Compare May 12, 2016 21:54

Bargs added 2 commits May 12, 2016 18:15

[Wizard] Creates a new CSV Add Data Wizard

57c391a

Remove pipeline creation step from CSV wizard

ed5e4e3

Bargs force-pushed the ingest/uploadUI branch from bed8725 to ed5e4e3 Compare May 12, 2016 22:16

Bargs assigned BigFunger May 25, 2016

BigFunger reviewed May 25, 2016
View reviewed changes

BigFunger assigned Bargs and unassigned BigFunger May 25, 2016

Bargs added 2 commits May 25, 2016 18:26

Extract complex error formatting logic into its own function

900a4ee

Added API test so we don't forget that the ID format is a part of the…

3143c06

… public contract

Bargs assigned BigFunger and unassigned Bargs May 25, 2016

BigFunger assigned Bargs and unassigned BigFunger May 26, 2016

Don't allow duplicate headers in CSVs

f237ec0

Bargs assigned w33ble and unassigned Bargs May 27, 2016

w33ble assigned Bargs and unassigned w33ble May 27, 2016

Bargs added 2 commits May 31, 2016 19:40

[FileUpload] Correctly detect the existence of directive attributes

d04aea5

Bargs assigned w33ble Jun 1, 2016

Truncate columns to 20 because we don't really need more, and perform…

b925554

…ance degrades if the user has hundreds or thousands

w33ble merged commit 94cc728 into elastic:feature/ingest Jun 1, 2016

Bargs mentioned this pull request Jun 1, 2016

[Pattern Review] Make the default pattern name a param on the directive #6749

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Data - CSV Upload UI #6845

Add Data - CSV Upload UI #6845

Bargs commented Apr 9, 2016 •

edited

Loading

rashidkpc commented Apr 14, 2016

rashidkpc commented Apr 14, 2016

tsullivan commented Apr 14, 2016

tsullivan commented Apr 15, 2016

tsullivan commented Apr 15, 2016 •

edited

Loading

tsullivan commented Apr 21, 2016

Bargs commented Apr 27, 2016

Bargs commented May 12, 2016

BigFunger May 25, 2016

BigFunger commented May 25, 2016

Bargs commented May 25, 2016

Bargs commented May 25, 2016

BigFunger commented May 26, 2016

BigFunger commented May 26, 2016

w33ble commented May 27, 2016

w33ble commented May 27, 2016

Bargs commented Jun 1, 2016

w33ble commented Jun 1, 2016

Add Data - CSV Upload UI #6845

Add Data - CSV Upload UI #6845

Conversation

Bargs commented Apr 9, 2016 • edited Loading

rashidkpc commented Apr 14, 2016

rashidkpc commented Apr 14, 2016

tsullivan commented Apr 14, 2016

tsullivan commented Apr 15, 2016

tsullivan commented Apr 15, 2016 • edited Loading

tsullivan commented Apr 21, 2016

Bargs commented Apr 27, 2016

Bargs commented May 12, 2016

BigFunger May 25, 2016

Choose a reason for hiding this comment

BigFunger commented May 25, 2016

Bargs commented May 25, 2016

Bargs commented May 25, 2016

BigFunger commented May 26, 2016

BigFunger commented May 26, 2016

w33ble commented May 27, 2016

w33ble commented May 27, 2016

Bargs commented Jun 1, 2016

w33ble commented Jun 1, 2016

Bargs commented Apr 9, 2016 •

edited

Loading

tsullivan commented Apr 15, 2016 •

edited

Loading